Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Applayer plugins final 5053 v3.18 #12363

Closed

Conversation

catenacyber
Copy link
Contributor

Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/5053

Describe changes:

  • app-layer plugins

Final step after #12352

Note that there is still #12307 to fix the limitation of probing parsers against 32 protocols (meaning any new app-layer like one in a plugin may be affected by this bug if it uses probing parsers for protocol detection)

Because some alprotos will remain static and defined as a constant,
such as ALPROTO_UNKNOWN=0, or ALPROTO_FAILED.

The regular already used protocols keep for now their static
identifier such as ALPROTO_SNMP, but this could be made more
dynamic in a later commit.

ALPROTO_FAILED was used in comparison and these needed to change to use
either ALPROTO_MAX or use standard function AppProtoIsValid
Ticket: 5053

The names are now dynamically registered at runtime.
The AppProto alproto enum identifiers are still static for now.

This is the final step before app-layer plugins.
@catenacyber catenacyber requested review from victorjulien, jasonish and a team as code owners January 9, 2025 07:56
@victorjulien
Copy link
Member

While it's good to see an example plugin, I will not merge the zabbix plugin as part of this work. We can consider that later, separately.

@victorjulien victorjulien marked this pull request as draft January 9, 2025 08:45
@catenacyber
Copy link
Contributor Author

While it's good to see an example plugin, I will not merge the zabbix plugin as part of this work. We can consider that later, separately.

@jasonish asked to add it in-tree for testing purposes

@catenacyber
Copy link
Contributor Author

#12364 without zabbix plugin

@catenacyber catenacyber closed this Jan 9, 2025
@jasonish
Copy link
Member

While it's good to see an example plugin, I will not merge the zabbix plugin as part of this work. We can consider that later, separately.

@jasonish asked to add it in-tree for testing purposes

I think I'd rather either:

  • A minimal template plugin just to excercise the main API points used (like the ci capture example plugin)
  • Or choose an existing app-layer and use this new API without making it a plugin (there is already a ticket for this, but doesn't need to be DNS: https://redmine.openinfosecfoundation.org/issues/4103)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants